Skip to content

feat: status page#16

Open
pthmas wants to merge 8 commits intomainfrom
pierrick/status-page
Open

feat: status page#16
pthmas wants to merge 8 commits intomainfrom
pierrick/status-page

Conversation

@pthmas
Copy link
Collaborator

@pthmas pthmas commented Feb 25, 2026

Overview

Adds a new /api/status endpoint returning chain ID, chain name, block height, total transactions, and total addresses, with chain name configurable via CHAIN_NAME env var. Renames the existing status endpoint to /api/height to better reflect its purpose, and adds a frontend status page displaying all chain information.

Summary by CodeRabbit

  • New Features

    • Added a Status page (accessible from the main nav) showing chain ID, chain name, block height, total transactions, total addresses, and last indexed timestamp.
    • Real-time updates: server-sent events for new blocks with a fast polling fallback for navbar counters; status data fetched on load and polled periodically.
  • Chores

    • Added CHAIN_NAME environment variable with a default for a human-readable chain name.

@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a lightweight /api/height endpoint for fast polling, enriches /api/status with chain metadata, fetches chain ID from RPC at startup into AppState (with CHAIN_NAME config), generalizes table counting, and adds a StatusPage in the frontend with polling and a status route.

Changes

Cohort / File(s) Summary
Configuration
\.env.example, docker-compose.yml, backend/crates/atlas-server/src/config.rs
Introduced CHAIN_NAME env var, defaulted in docker-compose and wired into backend Config.
AppState & Startup
backend/crates/atlas-server/src/api/mod.rs, backend/crates/atlas-server/src/main.rs
Added chain_id and chain_name to AppState; fetch_chain_id + parse_chain_id added and invoked at startup; tests for RPC chain ID retrieval added.
API Handlers - Status
backend/crates/atlas-server/src/api/handlers/status.rs
Added HeightResponse and new get_height handler; expanded ChainStatus to include chain_id, chain_name, total_transactions, total_addresses; get_status now populates those fields.
API Handlers - Counting
backend/crates/atlas-server/src/api/handlers/mod.rs, backend/crates/atlas-server/src/api/handlers/transactions.rs
Generalized get_table_count to accept table_name and added exact-count whitelisting/logic; transactions handler updated to call get_table_count(pool, "transactions").
Frontend API Client
frontend/src/api/status.ts
Added ChainStatusResponse interface, introduced HeightResponse for /height, changed getStatus to return height data, and added getChainStatus for full chain metadata.
Frontend Pages & Layout
frontend/src/components/Layout.tsx, frontend/src/pages/StatusPage.tsx, frontend/src/pages/index.ts, frontend/src/App.tsx
Added StatusPage component and export, registered /status route, added nav link; StatusPage fetches /api/status and polls /api/height every 5s with safe cleanup and renders status cards.
Docs / Notes
CLAUDE.md
Updated notes describing /api/height fast-poll, /api/status full metadata, and SSE/polling behavior for frontend.

Sequence Diagram(s)

sequenceDiagram
    participant Startup as Application Startup
    participant RPC as RPC Endpoint
    participant State as AppState
    participant DB as Database
    Startup->>RPC: fetch_chain_id (eth_chainId)
    RPC-->>Startup: chain ID (hex)
    Startup->>Startup: parse_chain_id(hex)
    Startup->>State: initialize state with chain_id, chain_name
    Startup->>DB: establish DB connections

    participant Client as StatusPage (Browser)
    participant API as Atlas API

    Client->>API: GET /api/status (getChainStatus)
    API->>State: read chain_id, chain_name
    API->>DB: query block_height, total_transactions, total_addresses
    API-->>Client: ChainStatusResponse
    Client->>Client: render status, start polling loop

    loop Every 5s
        Client->>API: GET /api/height (fast poll)
        API->>DB: read last_indexed_block
        API-->>Client: HeightResponse {block_height, indexed_at}
        Client->>Client: update UI block height
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped to fetch the chain’s clear name,
RPC returned its numeric fame.
A status once, a height to ping,
Five-second beats make live bells ring.
Hooray — the explorer knows its game!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title "feat: status page" accurately describes the main change: adding a new status page feature with accompanying API endpoints and configuration.
Docstring Coverage ✅ Passed Docstring coverage is 86.67% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pierrick/status-page
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pthmas pthmas force-pushed the pierrick/status-page branch from 6fcae3a to 8df58c9 Compare March 6, 2026 08:43
@pthmas pthmas marked this pull request as ready for review March 15, 2026 19:07
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/crates/atlas-api/src/handlers/status.rs (1)

27-63: ⚠️ Potential issue | 🟠 Major

Add same-file unit tests for the new status handler logic.

This file adds new response shapes and query flow but does not include a #[cfg(test)] mod tests block here.

As per coding guidelines "Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file. Run all tests with cargo test --workspace."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-api/src/handlers/status.rs` around lines 27 - 63, Add a
#[cfg(test)] mod tests block to this file containing unit tests for get_height
and get_status: mock or construct an Arc<AppState> with a test
sqlx::Sqlite/Postgres in-memory pool (or use sqlx::query mocking), insert a row
for key 'last_indexed_block', and assert that get_height returns the expected
HeightResponse and get_status returns ChainStatus with correct block_height,
indexed_at, total_transactions and total_addresses (exercise get_table_count by
creating test rows for "transactions" and "addresses" or mocking it). Name tests
clearly (e.g., test_get_height_returns_parsed_height,
test_get_status_returns_full_chain_status), import necessary types (AppState,
HeightResponse, ChainStatus, get_height, get_status, get_table_count) and run
them with cargo test --workspace to ensure compilation and behavior.
🧹 Nitpick comments (1)
backend/crates/atlas-api/src/handlers/mod.rs (1)

55-58: Consider identifier quoting for defense-in-depth.

The format! macro directly interpolates table_name into the SQL query. While all current callers pass hardcoded string literals (verified in status.rs and transactions.rs), adding identifier quoting would provide defense-in-depth against future misuse.

Optional: Quote the identifier
         // Exact count for small tables
-        let exact: (i64,) = sqlx::query_as(&format!("SELECT COUNT(*) FROM {table_name}"))
+        let exact: (i64,) = sqlx::query_as(&format!("SELECT COUNT(*) FROM \"{}\"", table_name))
             .fetch_one(pool)
             .await?;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-api/src/handlers/mod.rs` around lines 55 - 58, The query
interpolates table_name directly into sql and should quote and escape the
identifier for defense-in-depth: when building the sql for sqlx::query_as (where
variable table_name is used to produce exact), wrap the identifier in double
quotes and escape any embedded double quotes by replacing " with "" before
interpolation (or use sqlx's QueryBuilder/identifier-quoting helper if
available) so the final string becomes a safely quoted table identifier instead
of raw table_name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.env.example:
- Around line 4-6: The CHAIN_NAME value contains spaces and should be quoted;
update the .env.example entry for CHAIN_NAME to surround the value with double
quotes (e.g., CHAIN_NAME="My Chain") so dotenv-compatible loaders parse it
reliably and consistently.

In `@backend/crates/atlas-api/src/handlers/status.rs`:
- Around line 25-43: The /api/status contract was changed incorrectly — restore
get_status to be the lightweight endpoint that performs a single key-value
lookup against the indexer_state table and returns { block_height, indexed_at }
(the same shape as HeightResponse) and remove or rename the separate full-info
endpoint; specifically, move the SQL query "SELECT value, updated_at FROM
indexer_state WHERE key = 'last_indexed_block'" and its parsing logic (currently
in get_height) into get_status, have get_status return Json<HeightResponse>
using result.0.parse() for block_height and result.1.to_rfc3339() for
indexed_at, and delete or non-routify the get_height function/route so there is
only one /api/status endpoint complying with the guideline.

In `@frontend/src/App.tsx`:
- Around line 38-43: Routes for the "tokens" and "tokens/:address" entries have
inconsistent indentation compared to the other <Route> siblings; align the JSX
so all <Route ... /> elements in App.tsx use the same indentation level (e.g.,
match the indentation used by <Route path="nfts" ... />, <Route path="status"
... /> and others). Edit the lines containing the <Route path="tokens"
element={<TokensPage />} /> and <Route path="tokens/:address"
element={<TokenDetailPage />} /> entries to remove the extra leading space and
ensure consistent indentation across the Route list.

---

Outside diff comments:
In `@backend/crates/atlas-api/src/handlers/status.rs`:
- Around line 27-63: Add a #[cfg(test)] mod tests block to this file containing
unit tests for get_height and get_status: mock or construct an Arc<AppState>
with a test sqlx::Sqlite/Postgres in-memory pool (or use sqlx::query mocking),
insert a row for key 'last_indexed_block', and assert that get_height returns
the expected HeightResponse and get_status returns ChainStatus with correct
block_height, indexed_at, total_transactions and total_addresses (exercise
get_table_count by creating test rows for "transactions" and "addresses" or
mocking it). Name tests clearly (e.g., test_get_height_returns_parsed_height,
test_get_status_returns_full_chain_status), import necessary types (AppState,
HeightResponse, ChainStatus, get_height, get_status, get_table_count) and run
them with cargo test --workspace to ensure compilation and behavior.

---

Nitpick comments:
In `@backend/crates/atlas-api/src/handlers/mod.rs`:
- Around line 55-58: The query interpolates table_name directly into sql and
should quote and escape the identifier for defense-in-depth: when building the
sql for sqlx::query_as (where variable table_name is used to produce exact),
wrap the identifier in double quotes and escape any embedded double quotes by
replacing " with "" before interpolation (or use sqlx's
QueryBuilder/identifier-quoting helper if available) so the final string becomes
a safely quoted table identifier instead of raw table_name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2a6ab6a6-fbcc-4885-b4ae-d1effce5f286

📥 Commits

Reviewing files that changed from the base of the PR and between 47323e5 and 2950db9.

📒 Files selected for processing (12)
  • .env.example
  • CLAUDE.md
  • backend/crates/atlas-api/src/handlers/mod.rs
  • backend/crates/atlas-api/src/handlers/status.rs
  • backend/crates/atlas-api/src/handlers/transactions.rs
  • backend/crates/atlas-api/src/main.rs
  • docker-compose.yml
  • frontend/src/App.tsx
  • frontend/src/api/status.ts
  • frontend/src/components/Layout.tsx
  • frontend/src/pages/StatusPage.tsx
  • frontend/src/pages/index.ts

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
backend/crates/atlas-api/src/main.rs (2)

292-340: Good test coverage addition; add one more case for missing result.

You covered success, invalid hex, and transport failure. Add a test for the Line 38-40 path where result is absent/null to fully cover error branches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-api/src/main.rs` around lines 292 - 340, Add a new async
test in the tests module that uses serve_json_once to return a JSON-RPC response
missing the "result" field (e.g. {"jsonrpc":"2.0","id":1}) and assert that
fetch_chain_id(&url).await returns an error containing the expected message for
a missing/null result; name it something like
fetch_chain_id_returns_error_for_missing_result to match the existing test style
and ensure it covers the code path handling absent result in fetch_chain_id.

80-84: Add bounded retry for startup chain ID fetch.

Line 82 currently fails startup on the first transient RPC error. A short retry/backoff loop would improve boot reliability without changing fail-closed behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-api/src/main.rs` around lines 80 - 84, Wrap the single
call to fetch_chain_id(&rpc_url).await? in a bounded retry/backoff loop so
transient RPC errors don't fail startup immediately: perform N attempts (e.g.
3-5) with an increasing delay (e.g. exponential backoff using
tokio::time::sleep) between attempts, log each retry via tracing::info!/error!
including attempt count and error, and return the original error only if all
attempts are exhausted; update the startup block in main.rs where rpc_url and
fetch_chain_id are used to implement this retry logic around fetch_chain_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/crates/atlas-api/src/main.rs`:
- Around line 263-264: Remove the duplicate status surface by deleting the
.route("/api/height", get(handlers::status::get_height)) registration and ensure
the single chain-status handler handlers::status::get_status is the only exposed
endpoint; update handlers::status::get_status (and any related types) to return
exactly the contract shape { block_height, indexed_at } so clients rely solely
on GET /api/status for chain status information.

---

Nitpick comments:
In `@backend/crates/atlas-api/src/main.rs`:
- Around line 292-340: Add a new async test in the tests module that uses
serve_json_once to return a JSON-RPC response missing the "result" field (e.g.
{"jsonrpc":"2.0","id":1}) and assert that fetch_chain_id(&url).await returns an
error containing the expected message for a missing/null result; name it
something like fetch_chain_id_returns_error_for_missing_result to match the
existing test style and ensure it covers the code path handling absent result in
fetch_chain_id.
- Around line 80-84: Wrap the single call to fetch_chain_id(&rpc_url).await? in
a bounded retry/backoff loop so transient RPC errors don't fail startup
immediately: perform N attempts (e.g. 3-5) with an increasing delay (e.g.
exponential backoff using tokio::time::sleep) between attempts, log each retry
via tracing::info!/error! including attempt count and error, and return the
original error only if all attempts are exhausted; update the startup block in
main.rs where rpc_url and fetch_chain_id are used to implement this retry logic
around fetch_chain_id.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7471d202-a2de-49c1-ab17-523418b305c0

📥 Commits

Reviewing files that changed from the base of the PR and between 2950db9 and 15033ab.

📒 Files selected for processing (3)
  • .env.example
  • backend/crates/atlas-api/src/main.rs
  • frontend/src/App.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • .env.example
  • frontend/src/App.tsx

@pthmas pthmas requested a review from tac0turtle March 16, 2026 09:30
@pthmas pthmas self-assigned this Mar 16, 2026
/// Returns in <1ms, optimized for frequent polling.
pub async fn get_height(State(state): State<Arc<AppState>>) -> ApiResult<Json<HeightResponse>> {
let result: (String, chrono::DateTime<chrono::Utc>) = sqlx::query_as(
"SELECT value, updated_at FROM indexer_state WHERE key = 'last_indexed_block'",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is still blocked on items written to disk, if we batch write then its not really latest height but latest written height. what are your thoughts on doing a internal api on the indexer to talk to the api process to allow direct streaming before writing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll wait for the SSE refactoring to be merged and than we can read this value directly from the latest block in the buffer 👍

pthmas added 6 commits March 16, 2026 16:59
…ight

- Rename existing status endpoint to /api/height (block_height + indexed_at)
- Add new /api/status with chain_id, chain_name, total_transactions, total_addresses
- Fetch chain_id once from RPC at startup and store in AppState
- Read CHAIN_NAME from env, defaults to Unknown
- Make get_table_count accept a table_name parameter, reused for addresses
- Add StatusPage showing chain_id, chain_name, block_height, total_transactions, total_addresses
- Add getChainStatus() API call to /api/status
- Update getStatus() to call /api/height
- Add route and nav link for status page
@pthmas pthmas force-pushed the pierrick/status-page branch from 15033ab to 783aaf0 Compare March 16, 2026 16:04
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/crates/atlas-server/src/api/handlers/mod.rs (1)

20-57: 🛠️ Refactor suggestion | 🟠 Major

Add in-file unit tests for the new counting logic.

This introduces new branching behavior and should include #[cfg(test)] mod tests coverage in the same file (threshold path, parent fallback, and unsupported table handling if allowlisted).

As per coding guidelines "backend/**/*.rs: Add unit tests for new logic in a #[cfg(test)] mod tests block in the same file, and run with cargo test --workspace."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-server/src/api/handlers/mod.rs` around lines 20 - 57,
Add an in-file test module (#[cfg(test)] mod tests) under the same file that
contains async unit tests for get_table_count: one test that seeds/mocks
pg_class+pg_inherits to produce a large approx_partitions sum (>100_000) and
asserts get_table_count returns the approx value (threshold path), one test that
leaves no partition rows so the parent fallback path (query selecting reltuples
in parent) is used and the returned count matches the parent reltuples, and one
test for the "unsupported table handling if allowlisted" scenario described by
the review (mock or seed state to represent the allowlist and assert the
expected behavior). Locate tests next to get_table_count and reference the same
symbols (get_table_count, approx_partitions, parent) and ensure tests run with
async runtime and are executed by cargo test --workspace.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/status.rs (1)

34-34: Consider logging parse failures instead of silently defaulting to 0.

Using .unwrap_or(0) masks potential data corruption or schema issues in indexer_state.value. A block_height of 0 could be mistaken for "no blocks indexed yet" rather than "parse failed."

♻️ Proposed improvement
-    let block_height: i64 = result.0.parse().unwrap_or(0);
+    let block_height: i64 = result.0.parse().unwrap_or_else(|e| {
+        tracing::warn!("Failed to parse block height '{}': {}", result.0, e);
+        0
+    });

Also applies to: 50-50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-server/src/api/handlers/status.rs` at line 34, The
current parse of result.0 into block_height uses .unwrap_or(0), hiding parsing
errors; replace it with explicit parse handling that logs the parse failure
(including the offending string result.0 and the parse error) and only falls
back to a sentinel or returns an error as appropriate; update the let
block_height: i64 = result.0.parse().unwrap_or(0); line to use a match or
Result-based flow that calls the request/handler logger (or error macro used in
this module) to record the failure for indexer_state.value, and apply the same
change to the other occurrence mentioned (around lines 50) so parsing problems
aren’t silently turned into 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Line 52: Replace the unsafe dynamic identifier interpolation used in the
sqlx::query_as call (format!("SELECT COUNT(*) FROM {table_name}")) by
whitelisting allowed table names with a match on the table_name variable and
mapping each branch to a hard-coded, prebuilt query string (or prepared
statement) so the code never interpolates identifiers at runtime; update the
code paths that set the exact: (i64,) result and any approximate-count logic to
use the chosen query string. Also add a #[cfg(test)] mod tests block in the same
file (mod.rs) containing unit tests that exercise both the approximate-count
branch (simulate reltuples > 100_000) and the exact-count fallback branch
(simulate reltuples < 100_000), asserting the expected behavior of the function
that uses sqlx::query_as and the exact variable; use test doubles or a test
database fixture as appropriate to isolate the two scenarios.

---

Outside diff comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 20-57: Add an in-file test module (#[cfg(test)] mod tests) under
the same file that contains async unit tests for get_table_count: one test that
seeds/mocks pg_class+pg_inherits to produce a large approx_partitions sum
(>100_000) and asserts get_table_count returns the approx value (threshold
path), one test that leaves no partition rows so the parent fallback path (query
selecting reltuples in parent) is used and the returned count matches the parent
reltuples, and one test for the "unsupported table handling if allowlisted"
scenario described by the review (mock or seed state to represent the allowlist
and assert the expected behavior). Locate tests next to get_table_count and
reference the same symbols (get_table_count, approx_partitions, parent) and
ensure tests run with async runtime and are executed by cargo test --workspace.

---

Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/status.rs`:
- Line 34: The current parse of result.0 into block_height uses .unwrap_or(0),
hiding parsing errors; replace it with explicit parse handling that logs the
parse failure (including the offending string result.0 and the parse error) and
only falls back to a sentinel or returns an error as appropriate; update the let
block_height: i64 = result.0.parse().unwrap_or(0); line to use a match or
Result-based flow that calls the request/handler logger (or error macro used in
this module) to record the failure for indexer_state.value, and apply the same
change to the other occurrence mentioned (around lines 50) so parsing problems
aren’t silently turned into 0.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cea1f2a7-5571-4daa-8506-630104f6b7e9

📥 Commits

Reviewing files that changed from the base of the PR and between 15033ab and 783aaf0.

📒 Files selected for processing (14)
  • .env.example
  • CLAUDE.md
  • backend/crates/atlas-server/src/api/handlers/mod.rs
  • backend/crates/atlas-server/src/api/handlers/status.rs
  • backend/crates/atlas-server/src/api/handlers/transactions.rs
  • backend/crates/atlas-server/src/api/mod.rs
  • backend/crates/atlas-server/src/config.rs
  • backend/crates/atlas-server/src/main.rs
  • docker-compose.yml
  • frontend/src/App.tsx
  • frontend/src/api/status.ts
  • frontend/src/components/Layout.tsx
  • frontend/src/pages/StatusPage.tsx
  • frontend/src/pages/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • .env.example
  • frontend/src/pages/StatusPage.tsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 19-21: Remove the redundant `.into()` on the formatted string
passed to the `sqlx::Error::Protocol` constructor: the `format!("unsupported
table for exact count: {table_name}")` already yields a `String`, so update the
`Err(sqlx::Error::Protocol(...))` arm (the branch that returns the unsupported
table error) to pass the `String` directly to `sqlx::Error::Protocol` (leave the
`table_name` interpolation and surrounding message unchanged).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3416c1f9-2bb6-4801-a1c4-eace15a19cb7

📥 Commits

Reviewing files that changed from the base of the PR and between 783aaf0 and a98a585.

📒 Files selected for processing (1)
  • backend/crates/atlas-server/src/api/handlers/mod.rs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/crates/atlas-server/src/api/handlers/mod.rs (1)

34-69: ⚠️ Potential issue | 🟠 Major

Reject unsupported tables before the approximate-count branch.

Right now the whitelist only runs in the exact branch. An existing unsupported table above the threshold will return Ok(approx) instead of erroring, so get_table_count changes behavior based on row count. Validate table_name up front (or switch the parameter to an enum) so only the intended tables can use this helper.

🔧 Suggested fix
 pub async fn get_table_count(pool: &PgPool, table_name: &str) -> Result<i64, sqlx::Error> {
+    let exact_sql = exact_count_sql(table_name)?;
+
     // Sum approximate reltuples across partitions if any, else use parent.
     // This is instant and reasonably accurate for large tables.
     // Cast to float8 (f64) since reltuples is float4 and SUM returns float4
     let approx_partitions: (Option<f64>,) = sqlx::query_as(
         r#"
@@
     if should_use_approximate_count(approx) {
         Ok(approx)
     } else {
         // Exact count for small tables
-        let exact: (i64,) = sqlx::query_as(exact_count_sql(table_name)?)
+        let exact: (i64,) = sqlx::query_as(exact_sql)
             .fetch_one(pool)
             .await?;
         Ok(exact.0)
     }
 }
As per coding guidelines, `For large tables (transactions, addresses), use pg_class.reltuples for row count estimation instead of COUNT(*)`, with fallback to exact `COUNT(*)` for tables under 100k rows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/crates/atlas-server/src/api/handlers/mod.rs` around lines 34 - 69,
get_table_count currently only validates the allowed/unsupported table names in
the exact-count branch, so large unsupported tables can return Ok(approx); move
or add the whitelist validation at the start of get_table_count (or change the
function to accept a validated enum) so table_name is rejected immediately
before computing approx_partitions or calling should_use_approximate_count;
specifically, ensure the same whitelist check used in the exact branch is
invoked at the top of get_table_count (before calling approx_partitions query)
and return an error for unsupported names so only intended tables (e.g.,
transactions, addresses) can use reltuples-based estimation with fallback to
exact_count_sql when should_use_approximate_count decides to use COUNT(*).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 34-69: get_table_count currently only validates the
allowed/unsupported table names in the exact-count branch, so large unsupported
tables can return Ok(approx); move or add the whitelist validation at the start
of get_table_count (or change the function to accept a validated enum) so
table_name is rejected immediately before computing approx_partitions or calling
should_use_approximate_count; specifically, ensure the same whitelist check used
in the exact branch is invoked at the top of get_table_count (before calling
approx_partitions query) and return an error for unsupported names so only
intended tables (e.g., transactions, addresses) can use reltuples-based
estimation with fallback to exact_count_sql when should_use_approximate_count
decides to use COUNT(*).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf4ec5ca-bf79-4308-a4e5-f6e761f106b3

📥 Commits

Reviewing files that changed from the base of the PR and between a98a585 and 58ce533.

📒 Files selected for processing (1)
  • backend/crates/atlas-server/src/api/handlers/mod.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants